Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

properly deprecate the current behavior of the default_format option #1419

Merged
merged 15 commits into from
Feb 23, 2022

Conversation

keewis
Copy link
Contributor

@keewis keewis commented Nov 21, 2021

As discussed in #1407, this rolls back the change in behavior of default_format (split the spec in mspec and uspec and decide for each part individually if the default should be used).

I also decided that this would be a good opportunity to rewrite the code such that mspec and uspec are separated as early as possible, such that fixing #1413 becomes a really small change.

@keewis
Copy link
Contributor Author

keewis commented Nov 21, 2021

apparently, there's a issue with installing uncertainties==3.0.1.

Also, I just noticed that UnitRegistry.default_format does not have a documentation entry, and that that's actually a property that sets class attributes on Quantity, Unit, and Measurement.

@jules-ch
Copy link
Collaborator

Yes I will make PR to resolve the error for uncertainties

@jules-ch
Copy link
Collaborator

Should be okay if you rebase @keewis

@keewis keewis closed this Nov 21, 2021
@keewis keewis reopened this Nov 21, 2021
@keewis
Copy link
Contributor Author

keewis commented Dec 9, 2021

@hgrecco, this should be ready for reviewing and merging, after which we can issue the bugfix release we talked about in #1407 (maybe include #1422, too?)

@jules-ch
Copy link
Collaborator

jules-ch commented Dec 9, 2021

Gonna look into this. but looks good so far !

@jules-ch
Copy link
Collaborator

jules-ch commented Dec 9, 2021

@keewis can u rebase to fix conflicts

@keewis
Copy link
Contributor Author

keewis commented Jan 1, 2022

@hgrecco, @jules-ch, I think this should be ready for a final review. It would probably be good to include this, as well as #1422, in the bugfix release (which we should try to release soonish).

@keewis keewis mentioned this pull request Jan 16, 2022
21 tasks
@jules-ch
Copy link
Collaborator

jules-ch commented Feb 23, 2022

@keewis can you just change the condition that is duplicated to use if not mspec and default_mspec:
Idk if you received the notifications when I commented those lines.

@keewis
Copy link
Contributor Author

keewis commented Feb 23, 2022

@keewis can you just change the condition that is duplicated to use if not mspec and default_mspec:

sure

Idk if you received the notifications when I commented those lines.

I didn't (and I also can't see them anywhere), which is why I was confused when you posted #1450 (comment). Can you check if those comments are still pending and submit if so?

@jules-ch
Copy link
Collaborator

jules-ch commented Feb 23, 2022

image
They are still pending, can't you see them? Weird
I can make those changes directly if you want.

@keewis
Copy link
Contributor Author

keewis commented Feb 23, 2022

if it says "pending" you have to submit the review

@jules-ch
Copy link
Collaborator

jules-ch commented Feb 23, 2022

Oh I see, I thought those kind of review comments were automatically sent. My bad then.
Well I've just learned something it seems q_q

pint/formatting.py Outdated Show resolved Hide resolved
pint/quantity.py Outdated Show resolved Hide resolved
keewis and others added 2 commits February 23, 2022 22:26
Co-authored-by: Jules Chéron <43635101+jules-ch@users.noreply.github.com>
@hgrecco
Copy link
Owner

hgrecco commented Feb 23, 2022

I like this PR and it is a great idea to split uspec and mspec early on

@jules-ch jules-ch merged commit ff15a14 into hgrecco:master Feb 23, 2022
@keewis keewis deleted the deprecate-format-default branch February 23, 2022 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pint 0.18 does not allow to explicitly override "~" modifier in default_format
3 participants